fix(table-core): guard Array.isArray before index access in range filter autoRemove by JSap0914 · Pull Request #6353 · TanStack/table

JSap0914 · 2026-06-27T11:03:33Z

Bug

filterFn_between, filterFn_betweenInclusive, and filterFn_inNumberRange each contain:

autoRemove: (val: any) =>
  testFalsy(val) || (testFalsy(val[0]) && testFalsy(val[1]))

When a scalar (non-array) value such as 99 is passed, val[0] and val[1] are both undefined. testFalsy(undefined) === true, so the condition evaluates to true && true = true — the filter is immediately auto-removed even though 99 is a perfectly valid, non-empty filter value.

Numeric columns default to the inNumberRange filter function (auto-selected by column_getAutoFilterFn). Calling col.setColumnFilter(99) on a numeric column therefore silently discards the filter.

Closes #5949

Fix

Add Array.isArray(val) && before accessing val[0] / val[1], so the index checks only execute when val is actually an array. The leading testFalsy(val) still handles undefined, null, and ''.

// Before
autoRemove: (val: any) =>
  testFalsy(val) || (testFalsy(val[0]) && testFalsy(val[1]))

// After
autoRemove: (val: any) =>
  testFalsy(val) || (Array.isArray(val) && testFalsy(val[0]) && testFalsy(val[1]))

The same pattern is applied identically to all three range filter functions that shared the bug.

Verification

pnpm vitest run packages/table-core/tests/unit/fns/filterFns.test.ts

Before fix: 1 test fails — filterFns.inNumberRange.autoRemove > should NOT auto-remove a scalar number (autoRemove(99) returned true, expected false)

After fix: 358/358 tests pass (349 pre-existing + 9 new for filterFns.inNumberRange.autoRemove)

This fix was developed with AI assistance.

Summary by CodeRabbit

Bug Fixes
- Improved range filter cleanup logic to safely handle non-array values without incorrectly treating them as invalid.
- Preserved auto-removal for empty/missing range endpoints while keeping valid inputs (including 0) and scalar values from being dropped.
Tests
- Added unit tests covering inNumberRange auto-removal behavior for undefined/null/empty strings, empty endpoint arrays, and valid numeric ranges (including regression checks for scalar 99 and 0).

…oRemove fns filterFn_between, filterFn_betweenInclusive, and filterFn_inNumberRange all had: autoRemove: (val: any) => testFalsy(val) || (testFalsy(val[0]) && testFalsy(val[1])) When a scalar (non-array) value like `99` is passed, `val[0]` and `val[1]` are both `undefined`. testFalsy(undefined) === true, so the second branch evaluates to `(true && true)` === true and the filter is immediately removed — even though 99 is a perfectly valid, non-empty filter value. Root-cause: numeric columns default to the `inNumberRange` filter function (auto-selected in column_getAutoFilterFn). Calling `col.setColumnFilter(99)` hits this path and the filter silently vanishes (issue TanStack#5949). Fix: add `Array.isArray(val) &&` so the index checks only run when val is actually an array. The testFalsy(val) head still removes undefined/null/''. Closes TanStack#5949

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

coderabbitai · 2026-06-27T11:03:48Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f7e123e4-0f5a-4e01-bb89-f9e2378df34d

📥 Commits

Reviewing files that changed from the base of the PR and between 6a8e046 and 5a993df.

📒 Files selected for processing (2)

packages/table-core/src/fns/filterFns.ts
packages/table-core/tests/unit/fns/filterFns.test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

packages/table-core/src/fns/filterFns.ts
packages/table-core/tests/unit/fns/filterFns.test.ts

📝 Walkthrough

Walkthrough

Range filter autoRemove checks now guard against non-array values before reading range endpoints. Tests cover empty inputs, valid numeric ranges, and scalar regression cases for inNumberRange.

Changes

Range filter autoRemove safety

Layer / File(s)	Summary
Endpoint guard and tests `packages/table-core/src/fns/filterFns.ts`, `packages/table-core/tests/unit/fns/filterFns.test.ts`	`between`, `betweenInclusive`, and `inNumberRange` check `Array.isArray(val)` before endpoint access; tests cover empty, valid, and scalar `inNumberRange` inputs.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

TanStack/table#6332: Shares the range-filter autoRemove path and related endpoint-handling behavior.

Suggested reviewers: KevinVandy

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main fix: guarding range filter autoRemove with Array.isArray before indexing.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

⚔️ Resolve merge conflicts

Resolve merge conflict in branch fix/inNumberRange-autoRemove-non-array-value

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

🧹 Nitpick comments (1)

packages/table-core/tests/unit/fns/filterFns.test.ts (1)
566-601: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add the same scalar regression coverage for between and betweenInclusive.

This block protects inNumberRange, but the same Array.isArray fix was applied to the other two range filters too. Adding matching autoRemove(99) === false assertions there would keep all three call paths covered against the same regression.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/table-core/tests/unit/fns/filterFns.test.ts` around lines 566 - 601,
The scalar regression coverage was only added for
filterFns.inNumberRange.autoRemove, but the same Array.isArray fix applies to
filterFns.between.autoRemove and filterFns.betweenInclusive.autoRemove as well.
Update the existing test suite in filterFns.test.ts by adding matching scalar
assertions for those two autoRemove implementations, using the same 99 and 0
cases to ensure non-array values are not silently removed.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/table-core/tests/unit/fns/filterFns.test.ts`:
- Around line 566-601: The scalar regression coverage was only added for
filterFns.inNumberRange.autoRemove, but the same Array.isArray fix applies to
filterFns.between.autoRemove and filterFns.betweenInclusive.autoRemove as well.
Update the existing test suite in filterFns.test.ts by adding matching scalar
assertions for those two autoRemove implementations, using the same 99 and 0
cases to ensure non-array values are not silently removed.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a597de3a-0d3d-483a-834d-f0821b205763

📥 Commits

Reviewing files that changed from the base of the PR and between 0d07d77 and 6a8e046.

📒 Files selected for processing (2)

packages/table-core/src/fns/filterFns.ts
packages/table-core/tests/unit/fns/filterFns.test.ts

KevinVandy · 2026-06-29T13:54:41Z

wait, how is a number valid as an array?

The previous regression test for TanStack#6353 bundled the non-empty scalar case (autoRemove(99) and autoRemove(0)) with an empty-string expectation (autoRemove('')). testFalsy treats '' as falsy, so the leading testFalsy(val) branch correctly returns true for '' - the Array.isArray guard added in 0041efb is intentionally additive and not meant to override that behavior. Split the assertion: the non-empty scalar test now stands alone with just autoRemove(99) and autoRemove(0); the empty-string behavior is covered by a separate explicit assertion (expect(autoRemove(''))).toBe(true)) Vitest run: Test Files 1 passed (1) Tests 73 passed (73) Signed-off-by: sanjibani <18418553+sanjibani@users.noreply.github.com>

nx-cloud · 2026-07-12T16:51:21Z

View your CI Pipeline Execution ↗ for commit 6a8e046

Command	Status	Duration	Result
`nx affected --targets=test:eslint,test:sherif,t...`	⛔ Cancelled	32s	View ↗
`nx run-many --targets=build --exclude=examples/**`	✅ Succeeded	25s	View ↗

☁️ Nx Cloud last updated this comment at 2026-07-12 16:52:38 UTC

KevinVandy · 2026-07-12T18:45:53Z

There was a conflict in this one, but gave co-author credit to the merged PR

…ter autoRemove (TanStack#6394) * fix(table-core): guard Array.isArray before index access in range filter autoRemove `filterFn_between`, `filterFn_betweenInclusive`, and `filterFn_inNumberRange` shared an `autoRemove` that did `testFalsy(val[0]) && testFalsy(val[1])`. When a scalar value such as `99` was passed instead of a `[min, max]` tuple, `val[0]` and `val[1]` were both `undefined`, so the condition evaluated to `true && true` and the filter was silently auto-removed. Numeric columns default to `inNumberRange` via `column_getAutoFilterFn`, so calling `col.setColumnFilter(99)` on a numeric column discarded the filter before the row scan even ran. Wrap the index checks in `Array.isArray(val) &&`. The leading `testFalsy(val)` still handles `undefined`, `null`, `0`, and `''`. Behavior of the existing array-tuple cases is unchanged because `Array.isArray` is true and short-circuits to the same boolean. Closes TanStack#6353 * test(filterFns): assert empty-string scalar IS auto-removed The previous regression test for TanStack#6353 bundled the non-empty scalar case (autoRemove(99) and autoRemove(0)) with an empty-string expectation (autoRemove('')). testFalsy treats '' as falsy, so the leading testFalsy(val) branch correctly returns true for '' - the Array.isArray guard added in 0041efb is intentionally additive and not meant to override that behavior. Split the assertion: the non-empty scalar test now stands alone with just autoRemove(99) and autoRemove(0); the empty-string behavior is covered by a separate explicit assertion (expect(autoRemove(''))).toBe(true)) Vitest run: Test Files 1 passed (1) Tests 73 passed (73) Signed-off-by: sanjibani <18418553+sanjibani@users.noreply.github.com> --------- Signed-off-by: sanjibani <18418553+sanjibani@users.noreply.github.com> Co-authored-by: sanjibani <18418553+sanjibani@users.noreply.github.com> Co-authored-by: JSap0914

Copilot AI review requested due to automatic review settings June 27, 2026 11:03

Copilot AI reviewed Jun 27, 2026

coderabbitai Bot reviewed Jun 27, 2026

View reviewed changes

sanjibani mentioned this pull request Jul 9, 2026

fix(table-core): guard Array.isArray before index access in range filter autoRemove #6394

Merged

ci: apply automated fixes

5a993df

KevinVandy closed this in #6394 Jul 12, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(table-core): guard Array.isArray before index access in range filter autoRemove#6353

fix(table-core): guard Array.isArray before index access in range filter autoRemove#6353
JSap0914 wants to merge 2 commits into
TanStack:betafrom
JSap0914:fix/inNumberRange-autoRemove-non-array-value

JSap0914 commented Jun 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

coderabbitai Bot commented Jun 27, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

KevinVandy commented Jun 29, 2026

Uh oh!

nx-cloud Bot commented Jul 12, 2026 •

edited

Loading

Uh oh!

KevinVandy commented Jul 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

JSap0914 commented Jun 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bug

Fix

Verification

Summary by CodeRabbit

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

KevinVandy commented Jun 29, 2026

Uh oh!

nx-cloud Bot commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KevinVandy commented Jul 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JSap0914 commented Jun 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 27, 2026 •

edited

Loading

nx-cloud Bot commented Jul 12, 2026 •

edited

Loading